feat: highlighting of return values while the cursor is on match / if / =>
#19546
Conversation
rustbot
added
the
S-waiting-on-review
|
Here's a demo. 2025-04-09.10.32.29.mov |
roife
force-pushed
the
branch-value-highlights
branch
from
8fb7504 to
6d3a9e1
Compare
crates/ide/src/goto_definition.rs
Outdated
| fn nav_for_branches( | ||
| sema: &Semantics<'_, RootDatabase>, | ||
| token: &SyntaxToken, | ||
| ) -> Option<Vec<NavigationTarget>> { | ||
| let db = sema.db; | ||
|
|
||
| let navs = match token.kind() { | ||
| T![match] => sema | ||
| .descend_into_macros(token.clone()) | ||
| .into_iter() | ||
| .filter_map(|token| { | ||
| let match_expr = sema | ||
| .token_ancestors_with_macros(token) | ||
| .take_while(|node| !ast::MacroCall::can_cast(node.kind())) | ||
| .find_map(ast::MatchExpr::cast)?; | ||
| let file_id = sema.hir_file_for(match_expr.syntax()); | ||
| let focus_range = match_expr.match_token()?.text_range(); | ||
| let match_expr_in_file = InFile::new(file_id, match_expr.into()); | ||
| Some(expr_to_nav(db, match_expr_in_file, Some(focus_range))) | ||
| }) | ||
| .flatten() | ||
| .collect_vec(), | ||
|
|
||
| T![=>] => sema | ||
| .descend_into_macros(token.clone()) | ||
| .into_iter() | ||
| .filter_map(|token| { | ||
| let match_arm = sema | ||
| .token_ancestors_with_macros(token) | ||
| .take_while(|node| !ast::MacroCall::can_cast(node.kind())) | ||
| .find_map(ast::MatchArm::cast)?; | ||
| let match_expr = sema | ||
| .ancestors_with_macros(match_arm.syntax().clone()) | ||
| .find_map(ast::MatchExpr::cast)?; | ||
| let file_id = sema.hir_file_for(match_expr.syntax()); | ||
| let focus_range = match_arm.fat_arrow_token()?.text_range(); | ||
| let match_expr_in_file = InFile::new(file_id, match_expr.into()); | ||
| Some(expr_to_nav(db, match_expr_in_file, Some(focus_range))) | ||
| }) | ||
| .flatten() | ||
| .collect_vec(), | ||
|
|
||
| T![if] => sema | ||
| .descend_into_macros(token.clone()) | ||
| .into_iter() | ||
| .filter_map(|token| { | ||
| let if_expr = sema | ||
| .token_ancestors_with_macros(token) | ||
| .take_while(|node| !ast::MacroCall::can_cast(node.kind())) | ||
| .find_map(ast::IfExpr::cast)?; | ||
| let file_id = sema.hir_file_for(if_expr.syntax()); | ||
| let focus_range = if_expr.if_token()?.text_range(); | ||
| let if_expr_in_file = InFile::new(file_id, if_expr.into()); | ||
| Some(expr_to_nav(db, if_expr_in_file, Some(focus_range))) | ||
| }) | ||
| .flatten() | ||
| .collect_vec(), | ||
|
|
||
| _ => return Some(Vec::new()), | ||
| }; | ||
|
|
||
| Some(navs) | ||
| } | ||
|
|
There was a problem hiding this comment.
judging by the tests, this doesn't really do anything signifcant. Can we drop this?
There was a problem hiding this comment.
The purpose here is "when a user performs a goto-def on the match kw, server should navigate to the match kw itself."
This approach is more from a user experience perspective. In VS Code, if a user perform a goto-def on an identifier and the response is the identifier itself, VS Code would understand that this location is a definition. Therefore, VS Code will automatically trigger find-references.
This is very intuitive for users: when I perform ctrl + click on match kw, it will automatically trigger find-references to locate all branch exit points.
The current logic here is indeed quite complicated. I will try to simplify it to make it look more elegant.
roife
force-pushed
the
branch-value-highlights
branch
2 times, most recently
from
335d67b to
e23e442
Compare
| if ast::MacroCall::can_cast(node.kind()) { | ||
| break; | ||
| } |
There was a problem hiding this comment.
I added logic to handle match_ast! because it does not expand into match. When hovering on a => inside match_ast!, r-a will not stop and continues to look for match. If match_ast! is nested within a match, it incorrectly assumes that this => corresponds to the outer match. By adding a break here, we can prevent it from continuing to search at the boundary of the macro call.
There was a problem hiding this comment.
That sounds like the wrong way of fixing this, if th4e => in the macro call input is not downmapped to anything, we shouldn't handle the logic here in the first place then. So a better fix would be to check if after downmapping, the => token we have is not a direct child of a ast::TokenTree, if it is, it can't be from a match expression anyways. Or actually, phrased oppositely, we should verify if the parent of a => is a match arm, if it isn't we shouldn't trigger the relevant logic.
Veykril
added
S-waiting-on-author
…y `match`, `if`, or match arm arrow (`=>`)
roife
force-pushed
the
branch-value-highlights
branch
from
e23e442 to
6f81f9f
Compare
Inspired by #18924, a follow-up to #17542.
This PR adds a new feature that highlights the related return values when the cursor is on
match,if, or a match arm arrow (=>). This helps users quickly identify the different possible outcomes of control flow expressions.For
matchkeyword:matchkeyword itselfFor
=>(match arm arrow):For
ifkeyword:ifkeywords in theif-elsechainThis feature works with nested expressions and macros.